Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(template): Added maturin as an option for build-system #152

Merged
merged 24 commits into from
Jul 27, 2023
Merged

feat(template): Added maturin as an option for build-system #152

merged 24 commits into from
Jul 27, 2023

Conversation

ayeankit
Copy link
Contributor

@ayeankit ayeankit commented Jul 25, 2023 •
edited
Loading

Pull Request description

  1. Activate option in TUI to maturin
  2. Adding documentation about maturin
  3. Adding maturin as a build-system:
  4. Creating a maturin-pyproject.toml
  5. Editing post_gen_project.py
  6. Creating a smoke test (build-system.sh)
  7. Editing cookicutter.json
  8. Added maturin in Readme.md

Fixes #72

How to test these changes

  • ...

Pull Request checklists

This PR is a:

  • bug-fix
  • new feature
  • maintenance

About this PR:

  • it includes tests.
  • the tests are executed on CI.
  • the tests generate log file(s) (path).
  • pre-commit hooks were executed locally.
  • this PR requires a project documentation update.

Author's checklist:

  • I have reviewed the changes and it contains no misspelling.
  • The code is well commented, especially in the parts that contain more complexity.
  • New and old tests passed locally.

Additional information

Reviewer's checklist

Copy and paste this template for your review's note:

## Reviewer's Checklist

- [ ] I managed to reproduce the problem locally from the `main` branch
- [ ] I managed to test the new changes locally
- [ ] I confirm that the issues mentioned were fixed/resolved .

@xmnlab
Copy link
Member

xmnlab commented Jul 25, 2023

@ayeankit please fix the current issue on ci:

 
  × Preparing metadata (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [11 lines of output]
      error: failed to parse manifest at `/tmp/osl/osl-python-package/Cargo.toml`
      
      Caused by:
        invalid type: map, expected a string
        in `package.authors`
      💥 maturin failed
        Caused by: Cargo metadata failed. Does your crate compile with `cargo build`?
        Caused by: `cargo metadata` exited with an error:
      Error running maturin: Command '['maturin', 'pep517', 'write-dist-info', '--metadata-directory', '/tmp/pip-modern-metadata-pfgxmcny', '--interpreter', '/usr/share/miniconda3/envs/osl-python-package/bin/python3.10']' returned non-zero exit status 1.
      Checking for Rust toolchain....
      Running `maturin pep517 write-dist-info --metadata-directory /tmp/pip-modern-metadata-pfgxmcny --interpreter /usr/share/miniconda3/envs/osl-python-package/bin/python3.10`
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

https://github.com/osl-incubator/scicookie/actions/runs/5656756230/job/15324416571?pr=152#step:5:3757

@xmnlab
Copy link
Member

xmnlab commented Jul 25, 2023

@ayeankit please check the cookiecutter from scientific python
probably you will need to add some rust code as well, for example: https://github.com/scientific-python/cookie/blob/main/%7B%7Bcookiecutter.project_name%7D%7D/src/%7B%25%20if%20cookiecutter.backend%3D%3D'maturin'%20%25%7Dlib.rs%7B%25%20endif%20%25%7D

in there repo check all the code for maturin and also all files for maturin (with {% if cookiecutter.backend=='maturin' %} in the filename)

xmnlab
xmnlab reviewed Jul 27, 2023
View reviewed changes
src/scicookie/{{cookiecutter.project_slug}}/conda/dev.yaml Outdated
@@ -17,6 +17,8 @@ dependencies:
- pdm
{%- elif cookiecutter.build_system == "hatch" %}
- hatch
{%- elif cookiecutter.build_system == "maturin" %}
- maturin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably you will need to add rust as dependency here too

@ayeankit
Copy link
Contributor Author

Hey @xmnlab Finally all test cases passed!! Can you please review it once again?

@xmnlab
Copy link
Member

xmnlab commented Jul 27, 2023 •
edited
Loading

@ayeankit I just added some code style changes, let's just wait for the CI and it would be ready to go :)

@xmnlab xmnlab merged commit 4a6bfbd into osl-incubator:main Jul 27, 2023
11 checks passed
@github-actions
Copy link

github-actions bot commented Aug 9, 2023

🎉 This PR is included in version 0.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xmnlab xmnlab xmnlab left review comments

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for maturin build system (recommended for rust project)
2 participants
@ayeankit @xmnlab